Prevent overlapping referral program editions#1986
Conversation
🦋 Changeset detectedLatest commit: df1b847 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a shared base edition config type, enforces inclusive non‑overlapping time ranges per Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — the overlap detection algorithm is correct, all edge cases are tested, and the refactoring is clean. No P0 or P1 findings. The sort-then-adjacent-check algorithm correctly detects any interval overlap (standard interval scheduling proof applies). Inclusive-bound semantics are consistently implemented and tested across all three enforcement points (validateReferralProgramEditionConfigSet, the config-set transform, and the summaries superRefine). Fixtures in existing tests were correctly adjusted to stay disjoint under the new invariant. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Array of editions"] --> B["Group by chainId:address key"]
B --> C{"Group size >= 2?"}
C -- No --> G["Next group"]
C -- Yes --> D["Sort group by startTime"]
D --> E["Iterate adjacent pairs i, i-1"]
E --> F{"curr.startTime <= prev.endTime?"}
F -- Yes --> H["Return overlapping pair [prev, curr]"]
F -- No --> I{"More pairs?"}
I -- Yes --> E
I -- No --> G
G --> J{"More groups?"}
J -- Yes --> C
J -- No --> K["Return null (no overlap)"]
H --> L["Zod ctx.addIssue / throw Error"]
K --> M["Validation passes"]
Reviews (3): Last reviewed commit: "review" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Prevents a single referral from matching multiple referral program editions by enforcing a per-subregistryId time-range non-overlap invariant (with inclusive bounds), and refactors shared edition config fields/types to be reused across configs and summaries.
Changes:
- Introduces
BaseReferralProgramEditionConfigand uses it as a shared parent for edition configs and base edition summaries. - Adds
findOverlappingEditionPair/validateNonOverlappingEditionTimesand enforces the invariant in both config-set building and Zod parsing (including forward-compatible/unrecognized editions). - Adds/updates tests and fixtures to reflect inclusive-bound overlap behavior; exposes award-model Zod schemas via
@namehash/ens-referrals/internal.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ens-referrals/src/internal.ts | Re-exports per-award-model and shared Zod schemas via the internal entrypoint. |
| packages/ens-referrals/src/edition.ts | Adds shared base config type and overlap detection/validation; enforces invariant when building config sets. |
| packages/ens-referrals/src/edition.test.ts | New unit tests for overlap detection/validation and config-set building. |
| packages/ens-referrals/src/award-models/shared/edition-summary.ts | Refactors base summary to extend the shared base edition config type. |
| packages/ens-referrals/src/award-models/shared/api/zod-schemas.ts | Adds shared Zod schemas for edition slug and base edition config; reuses them in base summary schema. |
| packages/ens-referrals/src/api/zod-schemas.ts | Reuses shared schemas; enforces the non-overlap invariant in config-set array parsing and summaries response parsing. |
| packages/ens-referrals/src/api/zod-schemas.test.ts | Updates fixtures and adds test coverage for the non-overlap invariant (configs and summaries). |
| apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts | Updates test setup to avoid summaries collapsing to overlapping windows by ensuring per-edition rules propagate into mock leaderboards. |
| .changeset/shiny-tigers-wander.md | Changeset for rejecting overlapping editions invariant. |
| .changeset/quiet-mirrors-shine.md | Changeset for exposing per-award-model Zod schemas via internal entrypoint. |
| .changeset/bright-rivers-flow.md | Changeset for adding BaseReferralProgramEditionConfig as a shared parent type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Nice work! 1 small question only. Please take the lead to merge when ready 👍
| throw new Error(`Duplicate edition config slugs detected: ${duplicates.join(", ")}`); | ||
| } | ||
|
|
||
| validateNonOverlappingEditionTimes(configs); |
There was a problem hiding this comment.
Is there a special reason not to merge this idea into the existing validateReferralProgramEditionConfigSet function?
There was a problem hiding this comment.
No reason, just missed it. Thank you!
There was a problem hiding this comment.
Pull request overview
Prevents referral program editions from overlapping in time for the same subregistryId (inclusive bounds), tightening the referrals data model invariants and adding forward-compatible validation for both configs and summaries.
Changes:
- Introduces
BaseReferralProgramEditionConfigand reuses it across config + summary types. - Adds overlap detection (
findOverlappingEditionPair) and enforces it in config-set validation and Zod parsing (including unrecognized award models). - Adds/updates tests to cover overlap rejection and adjusts fixtures to be non-overlapping.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/ens-referrals/src/internal.ts | Re-exports additional award-model Zod schemas via the internal entrypoint. |
| packages/ens-referrals/src/edition.ts | Adds base config type + overlap detection/validation for edition config sets. |
| packages/ens-referrals/src/edition.test.ts | New unit tests for overlap detection and config set invariant enforcement. |
| packages/ens-referrals/src/award-models/shared/edition-summary.ts | Makes base summary reuse the new base config shape. |
| packages/ens-referrals/src/award-models/shared/api/zod-schemas.ts | Adds shared base config + slug schemas; refactors base summary schema. |
| packages/ens-referrals/src/api/zod-schemas.ts | Enforces non-overlap in config arrays and edition summaries responses. |
| packages/ens-referrals/src/api/zod-schemas.test.ts | Adds overlap-invariant tests and updates fixtures to remain valid under inclusive bounds. |
| apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts | Updates mock leaderboards to avoid violating the new summaries non-overlap invariant. |
| docs/ensnode.io/ensapi-openapi.json | Bumps documented OpenAPI version. |
| .changeset/shiny-tigers-wander.md | Changeset for rejecting overlapping editions. |
| .changeset/quiet-mirrors-shine.md | Changeset for exposing per-award-model Zod schemas via internal entrypoint. |
| .changeset/bright-rivers-flow.md | Changeset for adding BaseReferralProgramEditionConfig. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ens-referrals/src/edition.test.ts`:
- Around line 79-86: The test's regex in the expectation for
buildReferralProgramEditionConfigSet relies on a specific ordering of slugs ("a"
before "b") which makes it brittle; update the test to either (a) assert
ordering-independent presence by checking that both /"a"/ and /"b"/ appear in
the thrown message and that the subregistry id
(0x57f1887a8bf19b14fc0df6fd9b2acc9af147ea85) is present, or (b) derive the
expected ordering from findOverlappingEditionPair (using makePieSplitEdition to
build the inputs and calling findOverlappingEditionPair to determine prev/curr)
and then assert the exact tuple format in the error message — adjust the expect
in edition.test.ts to use one of these approaches so tests don't depend on an
implementation-specific ordering.
- Around line 37-102: Add a test covering a 3+ edition input where the
overlapping pair is non‑adjacent in the input order but adjacent after
sort-by-startTime: create editions like a = makePieSplitEdition("a",1000,1200),
b = makePieSplitEdition("b",2300,3000), c = makePieSplitEdition("c",1500,2500)
then call findOverlappingEditionPair([c, a, b]) and assert the returned pair is
the overlapping (c,b) with the expected slug order; mirror a similar scenario
for buildReferralProgramEditionConfigSet if desired to ensure it throws when
those two share a subregistry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 040d4ad0-4816-4317-aa6b-e49a400793b0
📒 Files selected for processing (2)
packages/ens-referrals/src/edition.test.tspackages/ens-referrals/src/edition.ts
Prevent overlapping referral program editions
closes: #1959
Summary
Why
Testing
Pre-Review Checklist (Blocking)